Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

always add current poisition as the first point in trajectory points #73

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

k-okada
Copy link
Member

@k-okada k-okada commented Jul 16, 2016

hi, currently, joint trajectoy action server behavios

if points is 1, add current position to the first point in trajectory points

others, assuming that user set current position to the first point in trajectory points

Is there any reason to do this? why not always put first point in trajectory points ?

c.f #55, #55

@IanTheEngineer
Copy link
Contributor

Yep. This change makes sense. I considered this behavior in the past as well, but never followed through. I'd be willing to merge if you retarget at the development branch.

@k-okada k-okada force-pushed the always_add_first_point branch from 3386356 to e1484b8 Compare July 23, 2016 09:15
@k-okada
Copy link
Member Author

k-okada commented Jul 23, 2016

ok, rebased

@@ -361,7 +361,7 @@ def _on_trajectory_action(self, goal):

dimensions_dict = self._determine_dimensions(trajectory_points)

if num_points == 1:
if num_points >= 1 and trajectory_points[0].time_from_start.to_sec() > 0:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be following? because it returns when num_points == 0.

if trajectory_points[0].time_from_start.to_sec() > 0:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks #74

@k-okada
Copy link
Member Author

k-okada commented Sep 26, 2016

@IanTheEngineer

Yep. This change makes sense. I considered this behavior in the past as well, but never followed through. I'd be willing to merge if you retarget at the development branch.

sorry, I missed this commend. Created new pull request -> #74

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants